feat(security): ROLE_DOMAIN_MANAGER, role hierarchy, domain-scoped voter (#84)#87
Open
martinydeAI wants to merge 6 commits into
Open
feat(security): ROLE_DOMAIN_MANAGER, role hierarchy, domain-scoped voter (#84)#87martinydeAI wants to merge 6 commits into
martinydeAI wants to merge 6 commits into
Conversation
Adds the authorisation foundation that ADR 006 builds on: - `App\Security\Roles` — three constants (`ROLE_USER`, `ROLE_DOMAIN_MANAGER`, `ROLE_ADMIN`) so controllers and templates stop typing the raw strings. - `role_hierarchy` in `security.yaml`: `ROLE_ADMIN` implies `ROLE_DOMAIN_MANAGER`, so site-wide admins manage every domain from the same surfaces a domain manager uses. - `App\Security\EmailDomain::of(User): ?string` — pure helper that extracts the lowercased domain from a user's email. Reused by the voter today and by the scoped user-management list query in #85. - `App\Security\Voter\ManageUserVoter` — supports the `MANAGE_USER` / `APPROVE_USER` / `BLOCK_USER` attributes against a `User` subject. Grants when the actor (a) holds `ROLE_DOMAIN_MANAGER`, (b) is `ROLE_ADMIN` (short-circuit across every domain), or (c) shares the lowercased email domain with the subject. The voter never reads the subject's `status` — identity state stays orthogonal to authorisation. Sequenced as PR 2 of the User management milestone plan. Independent of PR 1 (#45 + #83, #86) — branched from `develop` in parallel. PR 5 (#64 + #85) consumes the voter and the `EmailDomain` helper. Closes #84. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
Removed ADR reference
Corrected comment formatting in Roles.php.
martinyde
requested changes
Jun 19, 2026
Removed redundant details about authorization foundation and user-management features.
Per review on PR #87 - apply the test-comment convention from CLAUDE.md to the two new test files: each `test...` method now opens with a single-line "Tests ...", "Ensures ...", or "Verifies ..." comment naming what it asserts. Pure documentation change - no test logic touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Links to issues
Closes #84.
Description
Adds the authorisation foundation that the rest of the milestone
work builds on:
App\Security\Roles— constantsROLE_USER,ROLE_DOMAIN_MANAGER,ROLE_ADMINso controllers, voters, andtemplates stop typing the raw strings.
role_hierarchyinsecurity.yaml:ROLE_ADMINimpliesROLE_DOMAIN_MANAGER. One admin surface serves both site-wideadmins and domain-scoped approvers.
App\Security\EmailDomain::of(User): ?string— pure helper thatextracts the lowercased part after
@. Defensive: returnsnullfor missing email, missing
@, or trailing@. Reused by thevoter today and by the scoped user-management repository finder in
the upcoming feat: scoped user-management list view for admins and domain managers #85.
App\Security\Voter\ManageUserVoter— supports the attributesMANAGE_USER,APPROVE_USER,BLOCK_USERagainst aUsersubject. Grants when the actor (a) holds
ROLE_DOMAIN_MANAGER,(b) is
ROLE_ADMIN(short-circuit across every domain), or(c) shares the lowercased email domain with the subject. The voter
never reads
status— identity state stays orthogonal toauthorisation.
No existing tests modified — the
PR adds two new test files
(
tests/Unit/Security/Voter/ManageUserVoterTest.php,tests/Unit/Security/EmailDomainTest.php).Screenshot of the result
n/a — security plumbing, no UI.
Checklist
Verified locally:
task coding-standards-check— PHP CS Fixer, Twig CS Fixer,Prettier (YAML / JS / CSS), markdownlint, composer validate +
normalize — all green.
task test-coverage— 63 tests, 187 assertions, 100 %coverage.
Additional comments or questions
Symfony\Component\Security\Core\Authorization\Voter\Voter'svoteOnAttribute()signature in Symfony 8 carries an optionalfourth
?Vote $vote = nullargument; the override matches thatexact signature. The
Voteslot is unused here (we always answerwith a bool) — Symfony's vote-explanation feature is opt-in and
unrelated to ADR 006's decision flow.
Voter uses
AccessDecisionManagerInterface::decide()rather thaninspecting
User::getRoles()directly so that the configuredrole_hierarchyapplies —ROLE_ADMINimplyingROLE_DOMAIN_MANAGERis what makes the admin short-circuit workwithout two role assignments on the actor's user row.
Details - AI specificities
docs/adr/006-user-approval-and-account-state.md(Draft; lands via docs: add ADR 004 — user registration, approval, and account state #61). Specifically the "Domain manager — a
role, not a flag" section: ADR 006 deliberately models the
per-domain approver capability as a role on the existing
User.rolescolumn rather than a dedicated boolean. Scope (whichdomain the manager governs) is derived from the user's own email
rather than a relation.
fields PR 1 adds (
name,status) and doesn't change the Userconstructor signature. The voter reads
getEmail()andgetRoles()which exist on
developtoday.EmailDomain::of()is consumed again byUserRepository::findVisibleTo()in PR 5 (feat: scoped user-management list view for admins and domain managers #85).
IsGranted('MANAGE_USER', subject: $user)on the approval / block / list actions in PR 5 (feat: approval queue for domain managers #64 + feat: scoped user-management list view for admins and domain managers #85).
(
/admin/users) — added in PR 5 (feat: scoped user-management list view for admins and domain managers #85).ROLE_DOMAIN_MANAGER— separate concern,not currently tracked as an issue.
Domainentity. The ADR is explicit: only introduce one ifan organisation later needs to span multiple email domains or a
user must manage a domain other than their own email's. Don't
pre-build the relation.
AccessDecisionManagerInterfacemocks. A functional voter-on-controller test belongs in PR 5,
where the voter is actually invoked from a route.